[sc-71083] Scratching itches on the Model Evaluation Stores API#180
Conversation
Add getters for properties of DSSModelEvaluationFullInfo Document data drift parameters and results
|
This pull request has been linked to Shortcut Story #71083: Scratching itches on the Model Evaluation Stores API. |
lpenet
left a comment
lpenet
left a comment
There was a problem hiding this comment.
Nice job.
We may, and probably will, have discussions on style and naming, but this PR is great as it will make the drift computation API really usable.
Now, we can write code like:
import dataiku
from dataiku import pandasutils as pdu
import pandas as pd
from dataikuapi.dss.modelevaluationstore import DataDriftParams, PerColumnDriftParamBuilder
client = dataiku.api_client()
project = client.get_project("THEBEATLES")
mes = project.get_model_evaluation_store("lPAW1Keb")
model_evaluations = mes.list_model_evaluations()
# to be replaced by some clever selection, maybe on tags :-)
me1 = model_evaluations[0]
me2 = model_evaluations[1]
drift = me1.compute_data_drift(me2)
drift_model_result = drift.get_drift_model_result()
drift_model_accuracy = drift_model_result.get_drift_model_accuracy()
print("Value: {} < {} < {}".format(drift_model_accuracy.get_lower_confidence_interval(),
drift_model_accuracy.get_value(),
drift_model_accuracy.get_upper_confidence_interval()))
print("p-value: {}".format(drift_model_accuracy.get_pvalue()))
>>> Value: 0.430935018027 < 0.472897196262 < 0.515245812113
>>> p-value: 0.9027103181664773
# those results are strange, let's check sample sizes
print("Reference sample size: {}".format(drift_model_result.get_reference_sample_size()))
print("Current sample size: {}".format(drift_model_result.get_current_sample_size()))
...
drift.get_univariate_drift_result().get_raw()["Pclass"]
>>> {u'histogram': {u'binCountsCurrent': [887, 0, 0, 0, 0, 0, 0, 0, 0, 4],
u'binCountsReference': [887, 0, 0, 0, 0, 0, 0, 0, 0, 4],
u'binEdges': [0.0,
22.0,
44.0,
66.0,
88.0,
110.0,
132.0,
154.0,
176.0,
198.0,
220.0],
u'nbMissingValuesCurrent': 0,
u'nbMissingValuesReference': 0,
u'rowCountCurrent': 891,
u'rowCountReference': 891},
u'ksTestPvalue': 1.0,
u'ksTestStatistic': 0.0,
u'name': u'Pclass',
u'populationStabilityIndex': 0.0,
u'type': u'NUMERICAL',
u'wassersteinDistance': 0.0}
===> no way, this is categorical
# check columns handling
per_col_rep = drift.get_per_column_report()
for col_rep in per_col_rep:
print("col {} - default handling {} - actual handling {}".format(col_rep.get_name(), col_rep.get_default_column_handling(), col_rep.get_actual_column_handling()))
# recompute, with Pclass set as CATEGORICAL
drift = me1.compute_data_drift(me2,
DataDriftParams.from_params(
PerColumnDriftParamBuilder().with_column_drift_param("Pclass", "CATEGORICAL", True).build()
)
)
drift.get_univariate_drift_result().get_raw()["Pclass"]
# how is my last me data drifting from the train time test dataset of the model I first evaluated ?
me99.compute_data_drift(me1.get_evaluation_full_info().get_model_full_id())
Etc.
IMHO, a few things are still missing.
We should provide an access to the name and the labels, which are respectively in me.get_evaluation_full_info().get_raw()["details"]["userMeta"]["name"] and me.get_evaluation_full_info().get_raw()["details"]["userMeta"]["labels"].
My first move was to provide direct accessors to them, but, out of consistency with DSSTrainedModelDetails, base class of DSSTrainedPredictionModelDetails, we should go for a get_user_meta and a save_user_meta methods. Providing a clear access path to the labels may be important, as it is the mean for a user to really track datasets, models and evaluations according to her own ontology. There is already a ModelEvaluationsController#setModelMeta in the private API. We should add its equivalent to the public API.
Having an access to the full id of the evaluated model, with get_model_full_id makes me realize that the user has no easy mean to get a model version, whether be it an instance of DSSTrainedPredictionModelDetails or DSSTrainedClusteringModelDetails. It would be nice to add helpers allowing to easily retrieve those model versions, and accessors to the DSSSavedModel. Do not start working on this: we need to discuss it with @instanceofme and @fterrazzoni . If the user wants to compare performance metrics between the SMV and the ME, it may prove handy. It may be better to do that in another PR if pertinent.
Finally, I find the wording XXXReport for information on column handling misleading. It is not your choice : the thing is named "perColumnReport". But, as they are in fact some configuration of the way to handle columns for univariate analysis, I think that we should change the name ColumnReport to ColumnSettings and update related methods accordingly. We should consequently rename perColumnReport perColumnSettings and update the Java and HTML code (no JS AFAIK) accordingly, if @fterrazzoni agrees.
|
So:
=> wait for this. :-) There is a confusion cause by the fact that we improperly store the Me name in details.userMeta.name. We should stop doing this. We should add a name property directly on the ME and expose it/ le labels in userMeta are those of the evaluated model, when this is a DSS model. The labels of the ME are in evaluation.details.
=> we shall do this later, with the following card https://app.shortcut.com/dataiku/story/72344/add-a-method-in-the-public-api-to-easily-retrieve-an-smv-a-lab-model-or-a-me-from-an-id
=> @fterrazzoni just said me he agrees, so you can proceed. Thanks! |
Return raw data for get_raw in UnivariateDriftResult and add a property for getting per column data drift info.
|
Backend part of this PR : https://github.com/dataiku/dip/pull/13843 |
lpenet
left a comment
lpenet
left a comment
There was a problem hiding this comment.
LGTM+Tested.
So, my little example becomes:
import dataiku
from dataiku import pandasutils as pdu
import pandas as pd
from dataikuapi.dss.modelevaluationstore import DataDriftParams, PerColumnDriftParamBuilder
client = dataiku.api_client()
project = client.get_project("THEBEATLES")
mes = project.get_model_evaluation_store("zc5qZnjU")
mes.build()
mes.build()
model_evaluations = mes.list_model_evaluations()
# to be replaced by some clever selection, maybe on tags :-)
me1 = model_evaluations[0]
me2 = model_evaluations[1]
drift = me1.compute_data_drift(me2)
drift_model_result = drift.drift_model_result
drift_model_accuracy = drift_model_result.drift_model_accuracy
print("Value: {} < {} < {}".format(drift_model_accuracy.lower_confidence_interval,
drift_model_accuracy.value,
drift_model_accuracy.upper_confidence_interval))
print("p-value: {}".format(drift_model_accuracy.pvalue))
# those results are strange, let's check sample sizes
print("Reference sample size: {}".format(drift_model_result.get_raw()["referenceSampleSize"]))
print("Current sample size: {}".format(drift_model_result.get_raw()["currentSampleSize"]))
drift.univariate_drift_result.per_column_drift_data["Pclass"]
# check columns handling
per_col_settings = drift.per_column_settings
for col_settings in per_col_settings:
print("col {} - default handling {} - actual handling {}".format(col_settings.name, col_settings.default_column_handling, col_settings.actual_column_handling))
# recompute, with Pclass set as CATEGORICAL
drift = me1.compute_data_drift(me2,
DataDriftParams.from_params(
PerColumnDriftParamBuilder().with_column_drift_param("Pclass", "CATEGORICAL", True).build()
)
)
drift.univariate_drift_result.per_column_drift_data["Pclass"]
instanceofme
left a comment
instanceofme
left a comment
There was a problem hiding this comment.
Overall the diff LGTM, some details on parameter naming & doc
lpenet
left a comment
lpenet
left a comment
There was a problem hiding this comment.
LGTM + retested with latest changes
4 participants
SC 71083
Some quality of life improvements made on the model evaluation stores API (no backend modification):
Improve access to the model evaluation:
Document data drift parameters and results :